Use try_from_bytes in handle_file_urls #7652
Conversation
cla-bot
bot
added
the
cla-signed
crates/zed/src/open_listener.rs
Outdated
| @@ -131,7 +129,7 @@ impl OpenListener { | |||
| .flat_map(|url| url.strip_prefix("file://")) | |||
| .map(|url| { | |||
| let decoded = urlencoding::decode_binary(url.as_bytes()); | |||
| PathBuf::from(OsStr::from_bytes(decoded.as_ref())) | |||
| PathBuf::try_from_bytes(decoded.as_ref()).unwrap() | |||
There was a problem hiding this comment.
Note: try_from_bytes always returns Ok on [cfg(unix)].
There was a problem hiding this comment.
Because we don't control the input to this method, we should not panic if the bytes are invalid, but just ignore them. I think the following would be better:
| PathBuf::try_from_bytes(decoded.as_ref()).unwrap() | |
| .flat_map(|url| { | |
| let decoded = urlencoding::decode_binary(url.as_bytes()); | |
| PathBuf::try_from_bytes(decoded.as_ref()).log_err() | |
| }) |
|
I have no objections from merging that in if the reviewer finds it appropriate, but strongly urge you to consider adding Windows runners (GitHub seem to allow the standard ones: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#using-a-github-hosted-runner so we can do smth. like #7581). I would not expect the code to 100% compile under Windows now, but some of the crates may be already? |
|
@SomeoneToIgnore I think we've decided to wait on any big windows stuff until post linux; so let's not worry about CI. @kazatsuyu Thanks for working on this. In general we should avoid unwrap'ing on errors, left a suggestion in line, but otherwise good to go. |
|
Thanks! |
Reduce build error on Windows.
Release Notes: